Feature: make ecs more flexible #2019
Conversation
…ed but not loaded
maihde
force-pushed
the
feat-make-ecs-more-flexible
branch
from
77dfe53
to
46968b0
Compare
|
This PR is stale because it has been open for 60 days with no activity. |
scripts/generators/es_template.py
Outdated
| def generate( | ||
| ecs_nested: Dict[str, FieldNestedEntry], | ||
| ecs_version: str, | ||
| ecs_component_name_prefix: str, | ||
| out_dir: str, | ||
| mapping_settings_file: str, | ||
| template_settings_file: str | ||
| ) -> None: |
There was a problem hiding this comment.
Not really having context for this tool, adding a new argument like this in the middle seems like it might not be the most compatible way of doing this.. Usually I'd expect new arguments to show up in the kwargs section of the list and then have a reasonable default, which in this case looks like it should be ecs:
| def generate( | |
| ecs_nested: Dict[str, FieldNestedEntry], | |
| ecs_version: str, | |
| ecs_component_name_prefix: str, | |
| out_dir: str, | |
| mapping_settings_file: str, | |
| template_settings_file: str | |
| ) -> None: | |
| def generate( | |
| ecs_nested: Dict[str, FieldNestedEntry], | |
| ecs_version: str, | |
| out_dir: str, | |
| mapping_settings_file: str, | |
| template_settings_file: str, | |
| ecs_component_name_prefix: str = "ecs", | |
| ) -> None: |
There was a problem hiding this comment.
It would be useful to detail what improvements you made from a user's perspective, as this is not detailed on the PR description. The individual commit messages do give a short indication, but also per the contribution guide, could you please add to CHANGELOG.next.md?
|
This pull request allows users to
This allows ECS tooling to be utilized by more organizations for generating their component templates, and documentation. |
|
This PR is stale because it has been open for 60 days with no activity. |
|
Hi! We just realized that we haven't looked into this PR in a while. We're We're labeling this PR as Thank you for your contribution! |
|
👍 |
1 similar comment
|
👍 |
When using the ECS tooling for custom projects, there are a few minor changes that make would make the tooling more flexible, especially in cases where the ECS schema itself might be minimally used. I appreciate that the primary use case for ECS tooling is the ECS schema, but given that changes are minor and their is value of increased utility for the tooling I hope they could be considered for inclusion in the baseline.
I welcome discussion on the proposed changes.